Skip to content

Follow TURN 300 Try Alternate redirect for Realm connections - #29

Merged
extremeheat merged 3 commits into
PrismarineJS:masterfrom
Pix3lPirat3:fix/turn-alternate-server-realm
Sep 22, 2026
Merged

extremeheat merged 3 commits into
PrismarineJS:masterfrom
Pix3lPirat3:fix/turn-alternate-server-realm

Conversation

@Pix3lPirat3

@Pix3lPirat3 Pix3lPirat3 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Realm joins time out on the default Werift backend because Microsoft's TURN gateway replies to Allocate with 300 Try Alternate, which Werift 0.24.4 does not follow. Native WebRTC already handles this redirect.

For Werift clients only, pre-resolve IPv4 UDP TURN redirects before creating the peer and pass the resulting ICE configuration to unmodified Werift. The probe matches the remote endpoint and STUN transaction, validates response framing, ignores unrelated/malformed packets, and stops when the connection attempt closes or times out. TCP/TLS/IPv6 URLs and native WebRTC bypass it. Failed probes preserve the original URL.

Redirects are not cached: each connection gets a fresh result, avoiding permanent failures and stale relay assignments. Probes run concurrently with a three-second limit per endpoint, bounded by the connection attempt. Results remain local to their attempt rather than overwriting shared credentials.

Pixel verified the original proof of concept against a live Realm: join and reconnect both spawned on Werift, whereas the unpatched backend timed out. The subsequent review fixes still need the same live confirmation.

Validation at 70b69bb: lint, strict TypeScript, and 54 local runtime tests pass, including all four Werift/native backend combinations and local UDP regression tests for response matching, malformed packets, transport selection, repeat lookups, cancellation, and timeout/retry races. Remote CI runs on Linux, macOS, and Windows.

After this merges, bedrock-protocol's pinned node-nethernet dependency needs updating and integration testing. A proper Werift upstream fix remains the preferred long-term solution.

Original diagnosis and live proof of concept by @Pix3lPirat3; review fixes implemented by OpenAI Astra at the maintainers' request.

@extremeheat extremeheat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Pix3lPirat3 — the comparison against native WebRTC and the successful Realm join/reconnect make the diagnosis convincing. CI is green on all three platforms, and the four new URL/cache tests also pass locally. A temporary workaround through ICE configuration is reasonable, but I would fix these issues before merging this PoC:

  1. Validate the probe response before accepting a redirect (src/turnRedirect.js:47–55). The request transaction ID is discarded, and the first received datagram ends the probe without checking sender, message type, magic cookie, transaction ID, or declared message/attribute bounds. I reproduced acceptance of a fabricated 300 response sent by a different local UDP socket, with a zero cookie, zero transaction ID and invalid response type; it returned the supplied alternate address. Preserve the request ID, match the queried endpoint and Allocate response, check framing, and ignore unrelated/malformed datagrams until a valid response or timeout. This is ordinary STUN response matching, not optional extra validation. Add local UDP tests that actually exercise the probe; the current tests seed the cache and never run it.

  2. Cancel or discard stale pre-resolution (src/client.js:228). The new await occurs before a peer exists, so close() cannot dispose of anything yet. After it resolves, createOffer() still creates a peer without checking whether the client closed, negotiation timed out, or a newer attempt replaced it. I reproduced a peer-constructor call after client.close() using a silent local TURN endpoint and valid dummy TURN credentials. Tie the probe to the attempt's lifetime, recheck that attempt after awaiting, and keep rewritten ICE servers local to it instead of assigning an old asynchronous result back into this.credentials. Cover close and timeout/retry during probing.

  3. Respect the selected backend and TURN transport (src/client.js:225–228, src/turnRedirect.js:67–73). turn: also permits ?transport=tcp; this implementation probes it over UDP and can rewrite it using a UDP-only alternate. I reproduced a TCP URL being rewritten from the host:port cache. Skip TCP/TLS/unsupported address forms in this UDP-specific workaround, and apply it only when the selected backend is Werift (including auto resolving to Werift). Native currently also waits for and consumes the pre-resolution, so it is not actually unaffected. The existing REQUESTED-TRANSPORT=UDP attribute describes the relay allocation; it does not make the client-to-TURN transport UDP.

  4. Avoid permanent caching of failures and stale alternates (src/turnRedirect.js:70–79). A single timeout is cached as null for the entire process, so future Realm attempts never retry the lookup even after the network recovers. Positive results also never expire. At minimum do not cache probe failures; give successful redirects a bounded lifetime (and keep any cache bounded). Add a test where an initial failed probe is followed by a successful redirect.

Please retain the live Realm join/reconnect check after these changes. I also support a proper Werift upstream fix so this workaround can eventually disappear; we do not need to implement a general TURN client here.

Reviewed at 892aad0. These local checks establish the edge cases above; I have not independently repeated the authenticated Realm test.

— OpenAI Astra

@extremeheat
extremeheat dismissed their stale review September 22, 2026 09:29

The findings from this review are addressed by 70b69bb, which I implemented at the maintainer’s request. All 54 tests and Linux/macOS/Windows CI pass. Clearing my stale change request; this is not an independent approval of my follow-up changes.

@extremeheat

Copy link
Copy Markdown
Member

@Pix3lPirat3 I've pushed the review fixes directly to your PR branch as 70b69bb, at @extremeheat's request. Your original diagnosis and configuration-based approach are retained.

  • Only the selected Werift backend does pre-resolution; native and auto selecting native bypass it.
  • Only IPv4 UDP TURN URLs are probed. TCP/TLS/IPv6 stay unchanged.
  • Connected UDP filters the sender; the parser also matches the STUN transaction, cookie, Allocate error response and packet/attribute bounds. Invalid packets are ignored until a valid reply or timeout.
  • Close/timeout/retry aborts the lookup and prevents stale results from creating peers or overwriting credentials.
  • Removed the cache instead of adding expiry machinery. Each join resolves afresh, so neither transient failures nor outdated relay assignments persist. Probes run concurrently.

Lint, TypeScript and 54 tests pass, including local UDP tests exercising the actual probe and the cancellation races. Linux, macOS and Windows CI are green: https://github.com/PrismarineJS/node-nethernet/actions/runs/35710481348.

I cleared my previous change request because those findings are addressed; I authored the follow-up, so this is not a separate independent approval. When convenient, could you reconfirm Realm join/reconnect with this exact commit? We haven't repeated the authenticated live test ourselves. This PR is still open, and Bedrock's pin has not changed yet.

— OpenAI Astra

@extremeheat
extremeheat merged commit e5bb875 into PrismarineJS:master Sep 22, 2026
6 checks passed
@Pix3lPirat3

Copy link
Copy Markdown
Contributor Author

Reconfirmed on 70b69bb. Ran the authenticated live Realm test with node-nethernet at exactly 70b69bb (your review-fix commit), default Werift backend, against my Primary Realm:

  • Join: connect_allowed -> spawn in 32.3s
  • Reconnect (a fresh second connect; since this commit removed the cache it re-probes the TURN redirect each time): spawn in 11.4s

Both reach the world and spawn, so the validated probe + attempt-lifetime cancellation did not regress the Realm path.

One note in case it matters for the pin bump + integration CI: my Realm is on 1.26.51, so at bedrock-protocol's default 1.26.45 it reaches connect_allowed then kicks outdated_client (the TURN/NetherNet path connects fine; only the emulated version was behind) - I ran the above with version: '1.26.51'.

I did not exercise the native @roamhq/wrtc baseline in this checkout (not installed here), but per your review it is unaffected by this UDP-only, Werift-gated workaround.

@extremeheat

Copy link
Copy Markdown
Member

/makerelease patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants